Fix the styles compatibility hook for the editor iframes #40842
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
[Type] Bug
|
Size Change: +2 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
|
I can confirm that the fix resolves the issue for me when using
|
Test Report: Fix comment query Loop's duplicate inspector controlsEnv:
Steps to reproduce:
Before applying the PR on the default page:
Able to reproduce the issue ✅ After applying the PR:
Findings
|
|
I checked performance metrics and I see some concerning changes that we should investigate:
|
getdave
reviewed
May 5, 2022
There was a problem hiding this comment.
I tested this using the following within a test Plugin I made:
function myplugin_enqueue_block_editor_assets() {
wp_enqueue_style(
'davesmith-editor',
plugins_url( 'test-editor.css', __FILE__ ),
array( 'wp-edit-blocks' )
);
wp_enqueue_style(
'davesmith-block',
plugins_url( 'test-block.css', __FILE__ ),
array( 'wp-edit-blocks' )
);
}
add_action( 'enqueue_block_editor_assets', 'myplugin_enqueue_block_editor_assets' );The test-block.css file contains the following:
.wp-block-paragraph {
color: red !important;
}On trunk it doesn't work in the Site Editor (i.e. the paragraph block isn't red).
On this branch it works perfectly and the CSS is applied within the Site Editor iframe. See the screenshot and video.
Screen.Capture.on.2022-05-05.at.12-39-07.mp4
Great work 🙇 👏
|
I have just tested thoroughly with this fix + 5.9.3 and on 6.0 RC1. I have also tested on Chrome and Safari. Based on my testing, #40562 and #40469 are both resolved with this PR. Amazing work, thanks @youknowriad 🙌 Here are some examples using the Aino theme and Aino Blocks. Before:
Styles are not applied to the custom blocks in the patterns. After:
Styles are applied and patterns appear correctly. Here is another example using the Block Visibility plugin, which uses Before:
Contextual indicator is missing, it should appear on the Post Title. After:
Contextual indicator is now visible. |
ndiego
approved these changes
May 5, 2022
|
✅ Fixes issue with Do we need to add a note somewhere that only files that contain the block prefix ( @ndiego, what version of Aino Blocks are you using? I'm getting an error when trying to test with it. Cc @ellenbauer
|
I have exactly the same issue when opening the site editor with Aino theme installed and activated from the Theme Directory using the WP Admin interface. |
const BODY_CLASS_NAME = 'editor-styles-wrapper';
const BLOCK_PREFIX = 'wp-block';It's also styles that contain |
gziolo
approved these changes
May 5, 2022
@Mamaduka this was also the case before. I would love to see this removed in the future because it hampers extenders, but that is likely a separate PR. |
I am not seeing that error. I am seeing some block validation errors, but that is not related to this PR. I'm using the latest version of Aino and Aino Blocks plus the 6.0 RC1 and this PR. |
|
I haven't been able to check with the Aino plugin either, it breaks the editor as soon as I open the inserter but that seems separate. I'm pretty confident that this brings back the old behavior. |
This was referenced May 5, 2022
|
Thanks, everybody, for the quick turnaround ❤️ Can we backport this PR to Gutenberg 13.2 and release another RC? cc @georgeh |
georgeh
pushed a commit
that referenced
this pull request
May 5, 2022
|
13.2-rc2 has been published with this fix |
|
Thank you for such great work with fixing this. First Class! |
|
#40603 is confirmed fixed with this PR, awesome! |
|
I cherry-picked this PR for WordPress 6.0 RC2 release with b4fa92c. |
gziolo
added
[Package] Block editor
gziolo
pushed a commit
that referenced
this pull request
May 6, 2022
Based on feedback (see above) received in https://wordpress.slack.com/archives/C02QB2JS7/p1653450177962219 it might be that we need to find a way to amend this so that the styles end up in the |
|
@getdave @youknowriad Created a PR so these styles can end up in the head once again: #46732. |
This was referenced Aug 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #40562
closes #40469
What?
There was a regression related to the styles loading in the iframes (previews, site editor...) in Gutenberg 13.0.
I believe the regression happened as a result of this PR #38855 but the reality is that the bug pre-existed and was hidden.
How?
We had a function that was copying styles from the parent document to the iframe and using
appendChildto inject the cloned styles, the problem is that the parent element (the head element) was being controlled by React. Meaning its content can get wiped out at any moment due to re-rendering/remounting... (which I think has been triggered by #38855)This PR refactors how we apply the "cloning of styles" from the parent document by injecting these styles into a "div" at the top of the body and that div is "uncontrolled", meaning React doesn't care about its content. Also, I'm using
useRefEffectto retrigger the compatibility hook anytime the reference of that div changes. (It doesn't happen now but it's a security for the future).Testing Instructions
1- Add some stylesheet at the root of gutenberg call it
test.cssand put the following content.wp-block-group { background: red !important; }.2- Add the following code to the gutenberg.php file
3- Load the site editor and notice that group blocks have a red background.